Skip to content

internal: Move shims include so DISPATCH_INTERNAL_CRASH is defined #233

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Apr 21, 2017
Merged

internal: Move shims include so DISPATCH_INTERNAL_CRASH is defined #233

merged 1 commit into from
Apr 21, 2017

Conversation

amboar
Copy link
Contributor

@amboar amboar commented Mar 31, 2017

Fixes the build on powerpc64le under Linux.

Signed-off-by: Andrew Jeffery andrew@aj.id.au

@amboar
Copy link
Contributor Author

amboar commented Mar 31, 2017

@shahmishal Tagging you so you're aware for the PPC64LE Jenkins job.

@das
Copy link
Contributor

das commented Apr 3, 2017

can you explain more clearly what the problem is & for what use DISPATCH_INTERNAL_CRASH needs to be defined ?
the existing ordering here is intentional and your proposed re-ordering will not work as it breaks the build on Darwin

@das das self-requested a review April 3, 2017 06:52
das
das requested changes Apr 3, 2017
View reviewed changes
Copy link
Contributor

@das das left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the darwin conditionals following the #includes use the results (and sub-#includes) of shims.h and event_internal.h so this will not work

@amboar
Copy link
Contributor Author

amboar commented Apr 3, 2017

@das yes, this is the build issue I'm seeing on powerpc64le under Linux:

...
/bin/bash ../libtool  --tag=CXX   --mode=compile /home/ubuntu/swift-dev/build/buildbot_incremental/llvm-linux-powerpc64le/bin/clang++ -DHAVE_CONFIG_H -I. -I../config  -I.. -I.. -I../private -DDISPATCH_USE_DTRACE=0 -I../libpwq/include -Wall -fvisibility=hidden -momit-leaf-frame-pointer  -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -fblocks -I../src/BlocksRuntime -std=gnu++11 -fno-exceptions -O2 -c -o libdispatch_la-block.lo `test -f 'block.cpp' || echo './'`block.cpp
libtool: compile:  /home/ubuntu/swift-dev/build/buildbot_incremental/llvm-linux-powerpc64le/bin/clang++ -DHAVE_CONFIG_H -I. -I../config -I.. -I.. -I../private -DDISPATCH_USE_DTRACE=0 -I../libpwq/include -Wall -fvisibility=hidden -momit-leaf-frame-pointer -isystem /usr/include/bsd -DLIBBSD_OVERLAY -fblocks -I../src/BlocksRuntime -std=gnu++11 -fno-exce
ptions -O2 -c block.cpp  -fPIC -DPIC -o .libs/libdispatch_la-block.o
In file included from block.cpp:32:
In file included from ./internal.h:628:
In file included from ./shims.h:171:
./shims/lock.h:550:3: error: use of undeclared identifier 'DISPATCH_INTERNAL_CRASH'
                DISPATCH_INTERNAL_CRASH(errno, "sys_membarrier not supported");
                ^
1 error generated.
Makefile:701: recipe for target 'libdispatch_la-block.lo' failed
make[2]: *** [libdispatch_la-block.lo] Error 1
make[2]: Leaving directory '/home/ubuntu/swift-dev/swift-corelibs-libdispatch/src'
Makefile:541: recipe for target 'all' failed
make[1]: *** [all] Error 2
make[1]: Leaving directory '/home/ubuntu/swift-dev/swift-corelibs-libdispatch/src'
Makefile:457: recipe for target 'all-recursive' failed
make: *** [all-recursive] Error 1

In src/shims/lock.h:

DISPATCH_ALWAYS_INLINE
static inline dispatch_once_t
_dispatch_once_xchg_done(dispatch_once_t *pred)
{
#if defined(__i386__) || defined(__x86_64__)
        // On Intel, any load is a load-acquire, so we don't need to be fancy
        return os_atomic_xchg(pred, DLOCK_ONCE_DONE, release);
#elif defined(__linux__)
        if (unlikely(syscall(__NR_membarrier, MEMBARRIER_CMD_SHARED, 0) < 0)) {
                DISPATCH_INTERNAL_CRASH(errno, "sys_membarrier not supported");
        }
        return os_atomic_xchg(pred, DLOCK_ONCE_DONE, relaxed);
#else
#  error dispatch_once algorithm not available for this port
#endif
}

I guess an alternative is to move #define _dispatch_hardware_crash() ... up and call that directly in _dispatch_once_xchg_done(), as the #define for _dispatch_set_crash_log_cause_and_message() appears to do nothing? Clearly it's unacceptable to move the includes down as I have currently if it breaks Darwin - unfortunately I don't have a Darwin system to test against.

@MadCoder
Copy link
Contributor

MadCoder commented Apr 3, 2017

Just use __builtin_trap() here. And comment you can't use the define because of ordering.

@amboar
Copy link
Contributor Author

amboar commented Apr 3, 2017

@MadCoder thanks, I'll rework the change

@amboar
lock: Avoid use of undefined DISPATCH_INTERNAL_CRASH
d53fe63 
Building swift-corelibs-libdispatch on powerpc64le under Linux lead to
the following build failure:

	/bin/bash ../libtool  --tag=CXX   --mode=compile /home/ubuntu/swift-dev/build/buildbot_incremental/llvm-linux-powerpc64le/bin/clang++ -DHAVE_CONFIG_H -I. -I../config  -I.. -I.. -I../private -DDISPATCH_USE_DTRACE=0 -I../libpwq/include -Wall -fvisibility=hidden -momit-leaf-frame-pointer  -isystem /usr/include/bsd -DLIBBSD_OVERLAY  -fblocks -I../src/BlocksRuntime -std=gnu++11 -fno-exceptions -O2 -c -o libdispatch_la-block.lo `test -f 'block.cpp' || echo './'`block.cpp
	libtool: compile:  /home/ubuntu/swift-dev/build/buildbot_incremental/llvm-linux-powerpc64le/bin/clang++ -DHAVE_CONFIG_H -I. -I../config -I.. -I.. -I../private -DDISPATCH_USE_DTRACE=0 -I../libpwq/include -Wall -fvisibility=hidden -momit-leaf-frame-pointer -isystem /usr/include/bsd -DLIBBSD_OVERLAY -fblocks -I../src/BlocksRuntime -std=gnu++11 -fno-exce
	ptions -O2 -c block.cpp  -fPIC -DPIC -o .libs/libdispatch_la-block.o
	In file included from block.cpp:32:
	In file included from ./internal.h:628:
	In file included from ./shims.h:171:
	./shims/lock.h:550:3: error: use of undeclared identifier 'DISPATCH_INTERNAL_CRASH'
			DISPATCH_INTERNAL_CRASH(errno, "sys_membarrier not supported");
			^
	1 error generated.
	Makefile:701: recipe for target 'libdispatch_la-block.lo' failed
	make[2]: *** [libdispatch_la-block.lo] Error 1
	make[2]: Leaving directory '/home/ubuntu/swift-dev/swift-corelibs-libdispatch/src'
	Makefile:541: recipe for target 'all' failed
	make[1]: *** [all] Error 2
	make[1]: Leaving directory '/home/ubuntu/swift-dev/swift-corelibs-libdispatch/src'
	Makefile:457: recipe for target 'all-recursive' failed
	make: *** [all-recursive] Error 1

Include ordering in internal.h is tightly constrained, so open-code the macro
to avoid the dependency problem.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
@amboar
Copy link
Contributor Author

amboar commented Apr 10, 2017

I've reworked the commit and pushed - I wasn't sure whether to do that or apply a patch on top to fix (e.g. Rust's workflow). If I've taken the wrong approach please let me know.

@ffried ffried mentioned this pull request Apr 16, 2017
Closed
@ffried
Copy link

ffried commented Apr 16, 2017

This also fixes the build on RasPi. 👍

@das das merged commit 2df80a3 into swiftlang:master Apr 21, 2017
@ffried ffried mentioned this pull request Apr 21, 2017
Merged
das added a commit that referenced this pull request May 25, 2017
@das
Merge pull request #233 from amboar/master
b2789e6 
lock: Avoid use of undefined DISPATCH_INTERNAL_CRASH

Signed-off-by: Daniel A. Steffen <dsteffen@apple.com>
@das das mentioned this pull request May 25, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@das das das approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@amboar @das @MadCoder @ffried